Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add a flag to turn on/off the lowering of scalar broadcasting binary ops to NNPA #2778

Merged
merged 8 commits into from
Apr 3, 2024
Merged

Add a flag to turn on/off the lowering of scalar broadcasting binary ops to NNPA #2778

merged 8 commits into from
Apr 3, 2024

Conversation

tungld
Copy link
Collaborator

@tungld tungld commented Apr 1, 2024 •
edited
Loading

Add a compile flag, --nnpa-enable-scalar-bcast-binary, to turn on/off the lowering of scalar broadcasting binary ops to NNPA, which is flexible for debugging. Default value is off.

@tungld
Add a flag to turn on/off scalar broadcasting binary op in NNPA
6a49651 
Signed-off-by: Tung D. Le <tung@jp.ibm.com>
tungld and others added 4 commits April 1, 2024 03:56
@tungld
Fix a path
78b57c6 
Signed-off-by: Tung D. Le <tung@jp.ibm.com>
@AlexandreEichenberger
Merge branch 'main' into nnpa-flag-scalar-div
e6c63a0
@AlexandreEichenberger
Merge branch 'main' into nnpa-flag-scalar-div
e39a075
@tungld
fix linking
0d84453 
Signed-off-by: Tung D. Le <tung@jp.ibm.com>
@tungld tungld mentioned this pull request Apr 2, 2024
Closed
@tungld
fix linking errors
6c1658b 
Signed-off-by: Tung D. Le <tung@jp.ibm.com>
@tungld
Fix the linking errors
b70fa1a 
Signed-off-by: Tung D. Le <tung@jp.ibm.com>
@cjvolzka
Copy link
Collaborator

cjvolzka commented Apr 2, 2024

I gave this a try today and it looks like this closes the performance gap for roberta-sequence-classification-9 from #2769

chentong319
chentong319 approved these changes Apr 3, 2024
View reviewed changes
Copy link
Collaborator

@chentong319 chentong319 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

If you pass the option into the pass, the onnx-mlir-opt should work, too.

@tungld
Add a lit test
c1cd6f3 
Signed-off-by: Tung D. Le <tung@jp.ibm.com>
@tungld
Copy link
Collaborator Author

tungld commented Apr 3, 2024

@chentong319 thanks for the comment! I forgot to add a lit test. Since the option is added into OnnxMlirCommonOptions category, so it is available in onnx-mlir-opt also.

@tungld tungld merged commit 08d4fed into onnx:main Apr 3, 2024
8 checks passed
@jenkins-droid
Copy link
Collaborator

Jenkins Linux s390x Build #14619 [push] Add a flag to turn on/of... started at 23:57

@jenkins-droid
Copy link
Collaborator

Jenkins Linux amd64 Build #14589 [push] Add a flag to turn on/of... started at 22:57

@jenkins-droid
Copy link
Collaborator

Jenkins Linux ppc64le Build #13614 [push] Add a flag to turn on/of... started at 00:07

@jenkins-droid
Copy link
Collaborator

Jenkins Linux amd64 Build #14589 [push] Add a flag to turn on/of... passed after 1 hr 14 min

@jenkins-droid
Copy link
Collaborator

Jenkins Linux s390x Build #14619 [push] Add a flag to turn on/of... passed after 1 hr 36 min

@jenkins-droid
Copy link
Collaborator

Jenkins Linux ppc64le Build #13614 [push] Add a flag to turn on/of... passed after 2 hr 2 min

cjvolzka pushed a commit to cjvolzka/onnx-mlir that referenced this pull request Apr 3, 2024
@tungld @AlexandreEichenberger @cjvolzka
Add a flag to turn on/off the lowering of scalar broadcasting binary …
952de49 
…ops to NNPA (onnx#2778)

* Add a flag to turn on/off scalar broadcasting binary op in NNPA

Signed-off-by: Tung D. Le <tung@jp.ibm.com>

---------

Signed-off-by: Tung D. Le <tung@jp.ibm.com>
Co-authored-by: Alexandre Eichenberger <alexe@us.ibm.com>
cjvolzka pushed a commit to cjvolzka/onnx-mlir that referenced this pull request Apr 3, 2024
@tungld @cjvolzka
Add a flag to turn on/off the lowering of scalar broadcasting binary …
28338c4 
…ops to NNPA (onnx#2778)

* Add a flag to turn on/off scalar broadcasting binary op in NNPA

Signed-off-by: Tung D. Le <tung@jp.ibm.com>

---------

Signed-off-by: Tung D. Le <tung@jp.ibm.com>
Co-authored-by: Alexandre Eichenberger <alexe@us.ibm.com>
(cherry picked from commit 08d4fed)
@cjvolzka cjvolzka mentioned this pull request Apr 3, 2024
Merged
cjvolzka added a commit that referenced this pull request Apr 3, 2024
@cjvolzka @tungld
Add a flag to turn on/off the lowering of scalar broadcasting binary …
35a61d3 
…ops to NNPA (#2778) (#2782)

* Add a flag to turn on/off scalar broadcasting binary op in NNPA

Signed-off-by: Tung D. Le <tung@jp.ibm.com>

---------

Signed-off-by: Tung D. Le <tung@jp.ibm.com>
Co-authored-by: Alexandre Eichenberger <alexe@us.ibm.com>
(cherry picked from commit 08d4fed)

Co-authored-by: Tung D. Le <tung@jp.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chentong319 chentong319 chentong319 approved these changes

@AlexandreEichenberger AlexandreEichenberger Awaiting requested review from AlexandreEichenberger

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tungld @cjvolzka @jenkins-droid @chentong319 @AlexandreEichenberger